Skip to content

Conversation

@Danil-Grigorev
Copy link

@Danil-Grigorev Danil-Grigorev commented Apr 27, 2020

@Danil-Grigorev
Copy link
Author

/retest

Copy link
Contributor

@JoelSpeed JoelSpeed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve

@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JoelSpeed

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 17, 2020
@Danil-Grigorev
Copy link
Author

/retest

@enxebre
Copy link
Member

enxebre commented Jul 2, 2020

can we have a separate commit for each controller?
Is there already a counter part PR that it's leveraging these flags?

@enxebre
Copy link
Member

enxebre commented Jul 2, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 2, 2020
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@Danil-Grigorev
Copy link
Author

/hold missing changes for machineSet

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 2, 2020
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jul 2, 2020
@enxebre
Copy link
Member

enxebre commented Jul 2, 2020

@Danil-Grigorev thanks for changing the commits, that makes reviewing dramatically easier.
Could you please rephrase them to use imperative mood compliant with https://chris.beams.io/posts/git-commit/#imperative.
i.e Add Leader election flags for...

@Danil-Grigorev
Copy link
Author

@enxebre Feel free to LGTM

@enxebre
Copy link
Member

enxebre commented Jul 2, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 2, 2020
@Danil-Grigorev
Copy link
Author

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 3, 2020
@Danil-Grigorev
Copy link
Author

/retest

@enxebre
Copy link
Member

enxebre commented Jul 7, 2020

@Danil-Grigorev can we please also include the PR desc in the commit message 7ea5a32

@enxebre
Copy link
Member

enxebre commented Jul 13, 2020

/retest

@enxebre
Copy link
Member

enxebre commented Jul 13, 2020

/hold cancel
/retest

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 13, 2020
@enxebre
Copy link
Member

enxebre commented Jul 13, 2020

/hold
@dgrigore can you please include this message #571 (comment) in the commit which sets the flag?

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 13, 2020
Using leader election by default will add stronger guarantees than we have today that only one controller is running at a time to protect against edge cases where the deployment replica could be increased or upgrades with permissive maxSurge.

Relevant provider PRs:

- openshift/cluster-api-provider-gcp#85
- openshift/cluster-api-provider-aws#315
- openshift/cluster-api-provider-azure#122
- openshift/cluster-api-provider-openstack#108
- openshift/cluster-api-provider-baremetal#81
- openshift/cluster-api-provider-ovirt#55
- openshift#571
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jul 13, 2020
@enxebre
Copy link
Member

enxebre commented Jul 13, 2020

/lgtm
/hold cancel

@openshift-ci-robot openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Jul 13, 2020
@enxebre
Copy link
Member

enxebre commented Jul 13, 2020

RouteHealthDegraded: route not yet available, https://console-openshift-console.apps.ci-op-cmksplwg-bfd7b.origin-ci-int-aws.dev.rhcloud.com/health returns '503 Service Unavailable'"

/retest

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

2 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Jul 13, 2020

@Danil-Grigorev: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/e2e-azure-operator f0929b9 link /test e2e-azure-operator

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

1 similar comment
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit f4edd0e into openshift:master Jul 13, 2020
@stbenjam
Copy link
Member

@enxebre @Danil-Grigorev Why was this merged without the dependent cluster-api providers PR's all being merged in the description? This broke baremetal. Also, why wasn't e2e-metal-ipi run here?

It's an optional job, but it should be run on anything that affects us. If it's not going to get run before merging things like this, then maybe we need to enable it to run all the time.

@enxebre
Copy link
Member

enxebre commented Jul 15, 2020

sorry about that @stbenjam, I certainly thought the baremetal PR was merged. Please feel free to make that to job run on every PR while being not blocking.

andymcc added a commit to andymcc/cluster-api-provider-libvirt that referenced this pull request Jul 15, 2020
andymcc added a commit to andymcc/cluster-api-provider-libvirt that referenced this pull request Jul 15, 2020
andymcc added a commit to andymcc/cluster-api-provider-libvirt that referenced this pull request Jul 15, 2020
andymcc added a commit to andymcc/cluster-api-provider-libvirt that referenced this pull request Jul 15, 2020
This adds integration with:
openshift/machine-api-operator#571

Adding 3 new cli arguments for configuring leader elections:
-leader-elect
-leader-elect-lease-duration int
-leader-elect-resource-namespace string

Using leader election will add stronger guarantees than we have today
that only one controller is running at a time to protect against edge
cases where the deployment replica could be increased or upgrades with
permissive maxSurge.
elmiko pushed a commit to elmiko/machine-api-operator that referenced this pull request Aug 3, 2020
Using leader election by default will add stronger guarantees than we have today that only one controller is running at a time to protect against edge cases where the deployment replica could be increased or upgrades with permissive maxSurge.

Relevant provider PRs:

- openshift/cluster-api-provider-gcp#85
- openshift/cluster-api-provider-aws#315
- openshift/cluster-api-provider-azure#122
- openshift/cluster-api-provider-openstack#108
- openshift/cluster-api-provider-baremetal#81
- openshift/cluster-api-provider-ovirt#55
- openshift#571
andymcc added a commit to andymcc/cluster-api-provider-libvirt that referenced this pull request Oct 12, 2020
This adds integration with:
openshift/machine-api-operator#571

Adding 3 new cli arguments for configuring leader elections:
-leader-elect
-leader-elect-lease-duration int
-leader-elect-resource-namespace string

Using leader election will add stronger guarantees than we have today
that only one controller is running at a time to protect against edge
cases where the deployment replica could be increased or upgrades with
permissive maxSurge.

(cherry picked from commit 5ccc992)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. release/4.6

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants